Skip to content

feat: support custom additional information in PR welcome message#1147

Open
rnetser wants to merge 10 commits into
mainfrom
feat/issue-1143-welcome-extra-info
Open

feat: support custom additional information in PR welcome message#1147
rnetser wants to merge 10 commits into
mainfrom
feat/issue-1143-welcome-extra-info

Conversation

@rnetser

@rnetser rnetser commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

Add welcome-extra-info config option and .github-webhook-server-welcome-message.md file-based override to inject custom markdown content into PR welcome messages.

Changes

  • Schema (webhook_server/config/schema.yaml): welcome-extra-info (string, maxLength 10240) at global + per-repo level
  • Config loading (webhook_server/libs/github_api.py): Load from config via get_value(), async file loading from .github-webhook-server-welcome-message.md (10KB limit, UTF-8 validation), guarded by PR opened/reopened/ready_for_review and issue_comment events
  • Handler (webhook_server/libs/handlers/pull_request_handler.py): _prepare_extra_info_welcome_section property renders ### 📋 Additional Information section after Tips
  • Tests: 7 unit tests for file loading + 4 for section rendering
  • Examples (examples/config.yaml): Commented examples

Priority Chain (first match wins)

  1. .github-webhook-server-welcome-message.md file in repo
  2. .github-webhook-server.yaml welcome-extra-info
  3. config.yaml per-repo welcome-extra-info
  4. config.yaml global welcome-extra-info

Empty file or empty string explicitly suppresses inherited values.

E2E Test Results

# Test Result
1 Baseline — no config ✅ No section
2 Global config string ✅ Section rendered
3 Multi-line markdown ✅ Bold, lists, @mentions
4 Per-repo overrides global
5 Repo YAML override
6 File override (.md)
7 Empty file suppresses ✅ No section
8 /regenerate-welcome ✅ Section preserved
9 Empty YAML clears global ✅ No section

Closes #1143

@myakove-bot

Copy link
Copy Markdown
Collaborator

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: Disabled for this repository
  • Pre-commit Checks: pre-commit runs automatically if .pre-commit-config.yaml exists
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified
  • Labels: All label categories are enabled (default configuration)

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /hold cancel - Unblock PR merging
  • /verified - Mark PR as verified
  • /verified cancel - Remove verification status
  • /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
  • /regenerate-welcome - Regenerate this welcome message
  • /security-override - Set security check runs to pass (maintainers only)
  • /security-override cancel - Re-run security checks

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest build-container - Rebuild and test container image
  • /retest python-module-install - Test Python package installation
  • /retest pre-commit - Run pre-commit hooks and checks
  • /retest conventional-title - Validate commit message format
  • /retest all - Run all available tests

Container Operations

  • /build-and-push-container - Build and push container image (tagged with PR number)
    • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3
  • /cherry-pick-retry <branch> - Retry a failed cherry-pick (merged PRs only)

Branch Management

  • /rebase - Rebase this PR branch onto its base branch

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. LGTM Count: Minimum 1 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No wip, hold, has-conflicts labels and PR must be mergeable (no conflicts)
  5. Verified: PR must be marked as verified

📊 Review Process

Approvers and Reviewers

Approvers:

  • myakove
  • rnetser

Reviewers:

  • myakove
  • rnetser
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge
AI Features
  • Conventional Title: Mode: fix (claude/claude-opus-4-6-1m)
  • Cherry-Pick Conflict Resolution: Enabled (claude/claude-opus-4-6-1m)
  • Test Oracle: Triggers: approved (claude/claude-opus-4-6[1m]); /test-oracle can be used anytime
Security Checks
  • Suspicious Path Detection: Monitors paths: .claude/, .vscode/, .cursor/, .devcontainer/, .pi/, .github/workflows/, .github/actions/
  • Committer Identity Check: Verifies last committer matches PR author
  • Mandatory: Security checks block merge (use /security-override to bypass — maintainers only)

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is removed on new commits unless the push is detected as a clean rebase
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Container Builds: Container images are automatically tagged with the PR number
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Support custom “Additional Information” section in PR welcome messages

✨ Enhancement 🧪 Tests ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Add welcome-extra-info config option (global and per-repo) for markdown appended to welcome
 messages.
• Support repo file override via .github-webhook-server-welcome-message.md with size/UTF-8
 validation.
• Render a new “Additional Information” section in the PR welcome comment and add unit coverage.
Diagram

graph TD
  E{{"GitHub webhook event"}} --> P["GithubWebhook.process"] --> R["Resolve extra info"] --> H["PullRequestHandler"] --> C{{"Create welcome comment"}}
  Y["Config (welcome-extra-info)"] --> R
  F[(".github-webhook-server-welcome-message.md")] --> R
  subgraph Legend
    direction LR
    _ext{{"Event/External"}} ~~~ _svc["Service/Logic"] ~~~ _store[("Repo content")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Config-only extra info (no repo file)
  • ➕ Avoids an extra GitHub API call to fetch repo content
  • ➕ Keeps all customization centralized in the service config
  • ➖ Harder for repo owners to self-serve changes without changing shared config
  • ➖ Less discoverable than a repo-local markdown file
2. Read the markdown override from the cloned repo checkout
  • ➕ Avoids an additional GitHub contents API call (uses already-cloned files)
  • ➕ Keeps content retrieval consistent with other local-file processing
  • ➖ Requires cloning earlier/more often for events that currently don’t need it
  • ➖ May add I/O and lifecycle complexity around when the repo is available locally
3. Template-based welcome message customization
  • ➕ More structured customization (placeholders, conditional blocks)
  • ➕ Can prevent malformed markdown and enforce layout
  • ➖ Adds complexity and potential security/escaping issues
  • ➖ Requires documenting a templating language and maintaining compatibility

Recommendation: The chosen approach (simple markdown injection with a clear precedence chain and explicit “empty suppresses” semantics) is pragmatic and repo-owner friendly. If GitHub API call volume becomes a concern, consider switching the file override to be read from the local clone where feasible, but the current implementation is appropriate for this scope.

Files changed (6) +305 / -2

Enhancement (2) +76 / -0
github_api.pyResolve welcome extra info from config and optional repo markdown file +62/-0

Resolve welcome extra info from config and optional repo markdown file

• Introduces 'welcome_extra_info' field, loads it via 'config.get_value()', and conditionally fetches '.github-webhook-server-welcome-message.md' for relevant events. Adds file size enforcement (10KB), UTF-8 validation, and graceful fallback behavior on missing/invalid content.

webhook_server/libs/github_api.py

pull_request_handler.pyRender “Additional Information” section in welcome comment +14/-0

Render “Additional Information” section in welcome comment

• Appends a new markdown section after the Tips block when 'welcome_extra_info' is non-empty, injecting content as-is.

webhook_server/libs/handlers/pull_request_handler.py

Tests (2) +209 / -2
test_github_api.pyAdd unit tests for repo file loading and precedence behavior +110/-2

Add unit tests for repo file loading and precedence behavior

• Adds async tests covering success, missing file, oversized content, list responses, empty file suppression, unicode decode failure, and generic exception fallback while preserving config value when appropriate.

webhook_server/tests/test_github_api.py

test_pull_request_handler.pyAdd unit tests for welcome extra info section rendering +99/-0

Add unit tests for welcome extra info section rendering

• Adds tests asserting the section is omitted for empty/None values and rendered correctly for single and multi-line markdown content.

webhook_server/tests/test_pull_request_handler.py

Documentation (1) +6 / -0
config.yamlDocument 'welcome-extra-info' usage in example config +6/-0

Document 'welcome-extra-info' usage in example config

• Adds commented example YAML showing how to provide multi-line markdown for the new welcome extra info section.

examples/config.yaml

Other (1) +14 / -0
schema.yamlAdd 'welcome-extra-info' to global and per-repo schema +14/-0

Add 'welcome-extra-info' to global and per-repo schema

• Extends the config schema with a 'welcome-extra-info' string (10KB max) at both global and per-repo levels, documenting override and explicit-empty semantics.

webhook_server/config/schema.yaml

@qodo-code-review

qodo-code-review Bot commented Jul 1, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 13 rules

Grey Divider


Action required

1. welcome-extra-info tests accept invalid ✓ Resolved 📎 Requirement gap ≡ Correctness
Description
New tests assert that non-string and over-10KB welcome-extra-info values still load successfully,
which contradicts the requirement that schema validation reject invalid types/lengths. This weakens
automated detection of schema regressions for the new field.
Code

webhook_server/tests/test_config_schema.py[R1027-1075]

+    def test_welcome_extra_info_global_type_violation(
+        self, valid_minimal_config: dict[str, Any], monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """Test that welcome-extra-info with integer type still loads (runtime guard handles it)."""
+        config = valid_minimal_config.copy()
+        config["welcome-extra-info"] = 12345
+
+        temp_dir = self.create_temp_config_dir_and_data(config)
+
+        try:
+            monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_dir)
+
+            config_obj = Config()
+            assert config_obj.root_data["welcome-extra-info"] == 12345
+        finally:
+            shutil.rmtree(temp_dir)
+
+    def test_welcome_extra_info_repository_level(
+        self, valid_minimal_config: dict[str, Any], monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """Test valid welcome-extra-info at per-repo level."""
+        config = valid_minimal_config.copy()
+        config["repositories"]["test-repo"]["welcome-extra-info"] = "Repo-specific welcome info."
+
+        temp_dir = self.create_temp_config_dir_and_data(config)
+
+        try:
+            monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_dir)
+
+            config_obj = Config()
+            repo_data = config_obj.root_data["repositories"]["test-repo"]
+            assert repo_data["welcome-extra-info"] == "Repo-specific welcome info."
+        finally:
+            shutil.rmtree(temp_dir)
+
+    def test_welcome_extra_info_max_length(
+        self, valid_minimal_config: dict[str, Any], monkeypatch: pytest.MonkeyPatch
+    ) -> None:
+        """Test that welcome-extra-info exceeding 10240 chars loads (runtime guard handles it)."""
+        config = valid_minimal_config.copy()
+        config["welcome-extra-info"] = "x" * 10241
+
+        temp_dir = self.create_temp_config_dir_and_data(config)
+
+        try:
+            monkeypatch.setenv("WEBHOOK_SERVER_DATA_DIR", temp_dir)
+
+            config_obj = Config()
+            assert len(config_obj.root_data["welcome-extra-info"]) == 10241
Relevance

⭐⭐⭐ High

Team previously accepted strengthening schema tests to assert schema.yaml enforces types/structures
(#1112); tests asserting invalid values “still load” conflicts.

PR-#1112

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1523313 requires automated tests to verify schema rejects invalid types/structures
for welcome-extra-info. The added tests explicitly assert that an integer value and an over-limit
string are accepted/loaded, which fails that requirement.

Add schema validation tests for welcome-extra-info
webhook_server/tests/test_config_schema.py[1027-1041]
webhook_server/tests/test_config_schema.py[1062-1075]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new tests for `welcome-extra-info` currently assert that invalid values (non-string type and >10KB length) still load successfully. Per compliance, schema validation tests must ensure invalid configurations are rejected.

## Issue Context
- The schema defines `welcome-extra-info` as `type: string` with `maxLength: 10240`.
- The added tests currently encode the opposite expectation (that invalid type/length still loads), which prevents tests from catching schema validation regressions.

## Fix Focus Areas
- webhook_server/tests/test_config_schema.py[1027-1075]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. New tests lack type hints ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
Newly added pytest fixtures/tests introduce functions with untyped parameters and missing return
type annotations. This violates the requirement for full type annotations on all modified/added
function and method signatures.
Code

webhook_server/tests/test_github_api.py[R2820-2830]

+    @pytest.fixture()
+    def github_webhook(self, process_github_webhook):
+        """Create a GithubWebhook instance for welcome extra info tests."""
+        gw = process_github_webhook
+        gw.welcome_extra_info = "config value"
+        gw.repository = MagicMock()
+        return gw
+
+    @pytest.mark.asyncio()
+    async def test_load_from_file_success(self, github_webhook):
+        content_file = MagicMock()
Relevance

⭐⭐⭐ High

Type-annotation gaps in tests were previously flagged and accepted fixes (PR #968, #1123).

PR-#968
PR-#1123

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1523029 requires explicit type annotations for every parameter and an explicit return type. The
added fixture and async tests omit parameter annotations and return annotations.

Rule 1523029: Require full type annotations on all function and method signatures
webhook_server/tests/test_github_api.py[2820-2830]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New test functions/fixtures are missing parameter and return type annotations.

## Issue Context
Compliance requires explicit type annotations on all added/modified function and method signatures, including tests.

## Fix Focus Areas
- webhook_server/tests/test_github_api.py[2820-2922]
- webhook_server/tests/test_pull_request_handler.py[3301-3396]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. welcome-extra-info schema untested ✗ Dismissed 📎 Requirement gap ☼ Reliability
Description
The schema adds welcome-extra-info fields but no automated schema validation tests were added to
cover valid/invalid cases for the new field. This risks regressions where invalid configs may be
accepted (or valid configs rejected) without detection.
Code

webhook_server/config/schema.yaml[R254-260]

+  welcome-extra-info:
+    type: string
+    maxLength: 10240
+    description: |
+      Additional information to display at the end of the PR welcome message (global default).
+      Content is injected as-is (markdown). Can be overridden per-repo.
+      An empty string explicitly clears any inherited value.
Relevance

⭐⭐⭐ High

Team previously required adding tests when extending schema (accepted in PR #1112).

PR-#1112

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1523313 requires automated schema validation tests for the new welcome-extra-info field at
global and per-repo scope. The PR adds the field to the schema, but there is no corresponding test
coverage added/updated to validate it.

Add schema validation tests for welcome-extra-info
webhook_server/config/schema.yaml[254-260]
webhook_server/config/schema.yaml[533-539]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The schema introduces `welcome-extra-info` but the test suite does not add coverage validating correct schema behavior (valid types, maxLength, invalid types) for both global and per-repo scopes.

## Issue Context
A dedicated compliance requirement exists to add schema validation tests for this new field.

## Fix Focus Areas
- webhook_server/config/schema.yaml[254-260]
- webhook_server/config/schema.yaml[533-539]
- webhook_server/tests/test_config_schema.py[12-200]
- webhook_server/tests/test_schema_validator.py[47-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (2)
4. File markdown is .strip()ed ✗ Dismissed 📎 Requirement gap ≡ Correctness
Description
The markdown loaded from .github-webhook-server-welcome-message.md is modified via .strip(),
which can change user-provided content. This violates the requirement to inject the markdown content
as-is.
Code

webhook_server/libs/github_api.py[R1179-1183]

+            raw_content = await github_api_call(
+                lambda: content_file.decoded_content, logger=self.logger, log_prefix=self.log_prefix
+            )
+            decoded = raw_content.decode("utf-8").strip()
+            self.welcome_extra_info = decoded
Relevance

⭐⭐ Medium

No clear historical precedent about preserving markdown “as-is”; only tangential strip-related
guidance seen (PR #961).

PR-#961

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1523309 requires injecting user-provided markdown without modification. The loader decodes then
calls .strip(), which alters the content before it is rendered.

Render “Additional Information” section at end of PR welcome message (markdown injected as-is)
webhook_server/libs/github_api.py[1157-1183]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Loaded markdown must be used verbatim (as-is). The current implementation uses `.strip()`, which can remove intentional leading/trailing whitespace and newlines.

## Issue Context
Compliance requires that the selected additional-info markdown be injected without modification.

## Fix Focus Areas
- webhook_server/libs/github_api.py[1179-1183]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. _prepare_extra_info_welcome_section header wrong ✗ Dismissed 📎 Requirement gap ≡ Correctness
Description
The new Additional Information section uses a different header level/text (`### 📌 Additional
Information`) than required and is not appended at the very end of the welcome message. This breaks
the mandated output format for the welcome message rendering.
Code

webhook_server/libs/handlers/pull_request_handler.py[R695-707]

+    @property
+    def _prepare_extra_info_welcome_section(self) -> str:
+        """Prepare the Additional Information section for the welcome comment.
+
+        Renders user-provided extra information from configuration or
+        .github-webhook-server-welcome-message.md file.
+        Content is injected as-is (markdown).
+        """
+        if not self.github_webhook.welcome_extra_info:
+            return ""
+
+        return f"\n### 📌 Additional Information\n\n{self.github_webhook.welcome_extra_info}\n"
+
Relevance

⭐⭐ Medium

Welcome-message rendering correctness fixes have been accepted before (PR #890), but no precedent on
exact header level/text.

PR-#890

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 1523309 requires a #### Additional Information header and that the section be appended at the
end of the welcome message with markdown injected as-is. The implementation renders `### 📌
Additional Information` and places it before the final line of the welcome message.

Render “Additional Information” section at end of PR welcome message (markdown injected as-is)
webhook_server/libs/handlers/pull_request_handler.py[511-533]
webhook_server/libs/handlers/pull_request_handler.py[695-707]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The welcome message must render the Additional Information section exactly as required: header must be `#### Additional Information` and the section must be appended to the end of the welcome message.

## Issue Context
Current implementation renders `### 📌 Additional Information` and injects it before the trailing "For more information..." line, which violates the required format/placement.

## Fix Focus Areas
- webhook_server/libs/handlers/pull_request_handler.py[527-532]
- webhook_server/libs/handlers/pull_request_handler.py[695-707]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

6. Duplicate file fetch 🐞 Bug ➹ Performance ⭐ New
Description
GithubWebhook.process() preloads .github-webhook-server-welcome-message.md on pull_request
opened/ready_for_review, but PullRequestHandler.process_new_or_reprocess_pull_request() loads the
same file again when creating the welcome comment. This causes an avoidable second get_contents()
call on the common path where a welcome comment is created (including draft→ready_for_review).
Code

webhook_server/libs/github_api.py[R696-701]

+            if self.github_event == "pull_request" and self.hook_data.get("action") in (
+                "opened",
+                "ready_for_review",
+            ):
+                await self.load_welcome_extra_info_from_file()
+
Relevance

⭐⭐⭐ High

Team previously accepted refactors to eliminate duplicate API work/caching (e.g., avoid double user
enumeration, cache API lookups).

PR-#1117
PR-#906
PR-#1036

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
GithubWebhook.process() unconditionally calls load_welcome_extra_info_from_file() for
pull_request opened/ready_for_review, and the pull request handler calls it again when it
determines a welcome comment needs to be created, resulting in duplicate repository get_contents()
work on that common path.

webhook_server/libs/github_api.py[696-701]
webhook_server/libs/handlers/pull_request_handler.py[1975-1980]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The welcome-extra-info markdown file is fetched twice during welcome-comment creation for `pull_request` `opened` / `ready_for_review` events: once in `GithubWebhook.process()` and again in `PullRequestHandler.process_new_or_reprocess_pull_request()`.

### Issue Context
The handler already loads the file only when it is about to build a welcome message (and only when a welcome comment doesn’t exist). The earlier preload in `GithubWebhook.process()` is therefore redundant for these paths and costs an extra GitHub API call.

### Fix Focus Areas
- webhook_server/libs/github_api.py[696-701]
- webhook_server/libs/handlers/pull_request_handler.py[1975-1980]

### Implementation guidance
Choose one of:
1) Remove the preload from `GithubWebhook.process()` and rely on the handler(s) to call `load_welcome_extra_info_from_file()` only when building/regenerating the welcome message.
2) Add a small memoization flag on `GithubWebhook` (e.g., `welcome_extra_info_file_checked: bool`) so subsequent calls within the same webhook instance become no-ops.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Reopened loads unused file ✓ Resolved 🐞 Bug ➹ Performance
Description
GithubWebhook.process() eagerly calls load_welcome_extra_info_from_file() for pull_request
action reopened, but the reopened handler path does not build a welcome comment, so the file
fetch does not affect output. This adds unnecessary GitHub API traffic and latency on every reopen
event.
Code

webhook_server/libs/github_api.py[R696-701]

+            if self.github_event == "pull_request" and self.hook_data.get("action") in (
+                "opened",
+                "reopened",
+                "ready_for_review",
+            ):
+                await self.load_welcome_extra_info_from_file()
Relevance

⭐⭐⭐ High

Team often accepts reducing unnecessary webhook work/API calls in github_api.py (early-exit/skip
heavy ops in PRs 1039,1036).

PR-#1039
PR-#1036

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The webhook layer fetches the file for reopened, but the PR handler only creates a welcome comment
on opened/ready_for_review, and welcome_extra_info is only used when composing the welcome
comment, so the reopened fetch cannot change behavior.

webhook_server/libs/github_api.py[696-702]
webhook_server/libs/handlers/pull_request_handler.py[254-266]
webhook_server/libs/handlers/pull_request_handler.py[695-706]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`GithubWebhook.process()` loads `.github-webhook-server-welcome-message.md` for `pull_request` actions including `reopened`, but `PullRequestHandler` does not create a welcome comment on `reopened`, so the load is wasted.

### Issue Context
`welcome_extra_info` is only consumed when generating the welcome comment (`_prepare_extra_info_welcome_section`), and welcome comment creation is only performed for `opened` and `ready_for_review` in the `pull_request` handler.

### Fix Focus Areas
- webhook_server/libs/github_api.py[696-702]
- webhook_server/libs/handlers/pull_request_handler.py[254-266]

### Implementation notes
- Option A (minimal): Remove `"reopened"` from the action tuple in `GithubWebhook.process()`.
- Option B (cleaner ownership): Move the file-load to `PullRequestHandler.process_pull_request_webhook_data()` just before building/creating the welcome comment (only in the `opened`/`ready_for_review` branch), and remove the eager load from `GithubWebhook.process()` entirely.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Unneeded /reprocess file fetch ✓ Resolved 🐞 Bug ➹ Performance
Description
process_new_or_reprocess_pull_request() always calls load_welcome_extra_info_from_file() before
checking whether a welcome comment already exists, so /reprocess pays an avoidable repo
get_contents() call when the welcome comment is already present. This adds unnecessary latency and
consumes GitHub API rate limit without affecting output.
Code

webhook_server/libs/handlers/pull_request_handler.py[R1975-1977]

+        # Load file-based welcome extra info before building the welcome message
+        await self.github_webhook.load_welcome_extra_info_from_file()
+
Relevance

⭐⭐⭐ High

Team often accepts reducing unnecessary API work/early-exits (perf PRs like #906, #930; handler
optimizations accepted in #894).

PR-#906
PR-#930
PR-#894

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The handler loads extra info from the repo file before it checks whether the welcome comment exists;
when the comment exists, the method never builds/creates a welcome message, making the file load
unnecessary on that path.

webhook_server/libs/handlers/pull_request_handler.py[1967-1989]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`process_new_or_reprocess_pull_request()` loads `.github-webhook-server-welcome-message.md` unconditionally, even though the method only needs the extra info when it is about to *create* a welcome comment.

### Issue Context
This path is used by the `/reprocess` command. If a welcome comment already exists, the code skips creating a new welcome comment, but it still performs a repo contents fetch.

### Fix Focus Areas
- webhook_server/libs/handlers/pull_request_handler.py[1975-1986]

### Suggested change
Move `await self.github_webhook.load_welcome_extra_info_from_file()` inside the `if not await self._welcome_comment_exists(...)` branch (right before `_prepare_welcome_comment()`), so the file is fetched only when a welcome message will actually be created.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (5)
9. Schema byte/char mismatch ✗ Dismissed 🐞 Bug ≡ Correctness
Description
welcome-extra-info is constrained in the schema with maxLength: 10240, but runtime enforcement
uses UTF-8 byte length; multi-byte markdown can pass schema constraints yet be cleared at runtime
when it exceeds 10,240 bytes.
Code

webhook_server/config/schema.yaml[R109-112]

+  welcome-extra-info:
+    type: string
+    maxLength: 10240
+    description: |
Relevance

⭐⭐ Medium

Mixed evidence: accepts runtime guards (PR1109) but rejected schema/runtime alignment edits in
schema.yaml (PR1117).

PR-#1109
PR-#1117

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The schema sets maxLength: 10240, while runtime enforcement compares len(value.encode('utf-8'))
to 10240 bytes, so multi-byte content can satisfy the schema but still be rejected at runtime.

webhook_server/config/schema.yaml[109-115]
webhook_server/libs/github_api.py[1123-1138]
webhook_server/libs/github_api.py[66-67]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The schema encodes a 10,240 limit via `maxLength`, but runtime enforcement treats the limit as UTF-8 bytes. This can cause configuration values containing multi-byte characters to pass schema checks yet be ignored at runtime (welcome section unexpectedly missing).

## Issue Context
- JSON Schema `maxLength` is character-count based.
- Runtime enforcement for `welcome-extra-info` is explicitly byte-based (`encode('utf-8')`).

## Fix Focus Areas
- webhook_server/config/schema.yaml[109-115]

## Suggested fix
Choose one and make it explicit/consistent:
1) **Preferred:** Update the schema description to explicitly state the limit is **10,240 UTF-8 bytes enforced at runtime**, and keep `maxLength` only as an approximate editor hint (or remove `maxLength` to avoid implying strict enforcement).
2) Alternatively, change runtime enforcement to match schema semantics (character length) if byte-precision is not required.

Also update/adjust schema tests if they currently imply `maxLength` fully enforces the 10KB byte limit.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


10. Hardcoded container_repository_password test value ✗ Dismissed 📘 Rule violation ⛨ Security
Description
_create_mock_github_webhook() sets container_repository_password to the literal test-password,
which looks like a credential and violates the no-hardcoded-secrets requirement. Even in tests,
secrets/credentials should be sourced from environment/fixtures or replaced with clearly non-secret
placeholders.
Code

webhook_server/tests/test_pull_request_handler.py[77]

+    mock_webhook.container_repository_password = "tes*********d"  # pragma: allowlist secret
Relevance

⭐⭐ Medium

No clear precedent; token-like test literals w/ allowlist pragmas appear, but acceptance history is
unclear/mixed (PR 1081).

PR-#1081

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1523240 requires that secrets/credentials not be hardcoded in source (including
tests). The changed test helper assigns a password literal directly to
container_repository_password.

Rule 1523240: Do not hardcode secrets; read them from environment or secret manager
webhook_server/tests/test_pull_request_handler.py[77-77]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A password-like secret is hardcoded in a test helper (`container_repository_password = "tes*********d"`). This violates the requirement to not hardcode secrets in the codebase (including tests).

## Issue Context
The value is used to represent container registry credentials; even though it is likely a dummy value, it appears credential-like and is explicitly allowlisted as a secret.

## Fix Focus Areas
- webhook_server/tests/test_pull_request_handler.py[77-77]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


11. Size limit uses characters ✓ Resolved 🐞 Bug ≡ Correctness
Description
In _repo_data_from_config(), the welcome-extra-info 10KB guard uses len(str) (characters) but
logs “bytes”, so multibyte UTF-8 content can exceed 10KB while passing validation and the warning
message is misleading.
Code

webhook_server/libs/github_api.py[R1125-1134]

+        if not isinstance(self.welcome_extra_info, str):
+            self.logger.warning(
+                f"welcome-extra-info must be a string, got {type(self.welcome_extra_info).__name__}. Ignoring."
+            )
+            self.welcome_extra_info = ""
+        elif len(self.welcome_extra_info) > 10240:
+            self.logger.warning(
+                f"welcome-extra-info exceeds 10KB limit ({len(self.welcome_extra_info)} bytes). Ignoring."
+            )
+            self.welcome_extra_info = ""
Relevance

⭐⭐ Medium

Mixed history: accepted fixes for misleading config warnings in github_api.py (#998) but similar
“misleading log” fixes rejected (#1127).

PR-#998
PR-#1127

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The config guard uses character length and labels it as bytes, while the file-based loader enforces
size using a byte count, demonstrating an inconsistency and a real under-enforcement risk for UTF-8
multibyte content.

webhook_server/libs/github_api.py[1121-1134]
webhook_server/libs/github_api.py[1183-1188]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`welcome-extra-info` is documented and treated elsewhere as a 10KB limit, but the config guard checks `len(string)` (characters) and logs that value as "bytes". This can allow values over 10KB when encoded as UTF-8 and produces misleading warnings.

## Issue Context
The file-based override path enforces size in bytes (`content_file.size`), indicating the limit is intended to be byte-based.

## Fix Focus Areas
- webhook_server/libs/github_api.py[1125-1134]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


12. Eager file fetch on comments ✓ Resolved 🐞 Bug ➹ Performance
Description
GithubWebhook.process() loads .github-webhook-server-welcome-message.md for every issue_comment
event before parsing commands, causing unnecessary GitHub API calls even when the comment will not
regenerate/create a welcome message.
Code

webhook_server/libs/github_api.py[R694-699]

+            if self.github_event == "issue_comment" or (
+                self.github_event == "pull_request"
+                and self.hook_data.get("action") in ("opened", "reopened", "ready_for_review")
+            ):
+                await self._load_welcome_extra_info_from_file()
+
Relevance

⭐⭐ Medium

Some “avoid extra API work” changes accepted (PR #1039), but other perf refactors rejected (PR
#1109).

PR-#1039
PR-#1109

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new conditional in process() triggers file loading for any issue_comment event, while the file
loader performs a get_contents API call. The issue_comment handler skips edited/deleted actions and
only regenerates the welcome message for specific commands, meaning the eager fetch is unnecessary
for most comments.

webhook_server/libs/github_api.py[694-699]
webhook_server/libs/github_api.py[1157-1181]
webhook_server/libs/handlers/issue_comment_handler.py[78-85]
webhook_server/libs/handlers/issue_comment_handler.py[323-337]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The welcome-message markdown file is fetched for all `issue_comment` events, even though it is only needed when an issue comment command will regenerate or create a welcome message (e.g. `/regenerate-welcome`, and potentially `/reprocess` when it can create a welcome comment). This adds avoidable API traffic and increases rate-limit pressure.

### Issue Context
- The file loader uses `repository.get_contents(...)` which is a GitHub API call.
- In the issue_comment handler, only some commands actually regenerate/create the welcome message.

### Fix Focus Areas
- webhook_server/libs/github_api.py[694-699]
- webhook_server/libs/github_api.py[1157-1181]
- webhook_server/libs/handlers/issue_comment_handler.py[73-120]
- webhook_server/libs/handlers/issue_comment_handler.py[323-337]
- webhook_server/libs/handlers/pull_request_handler.py[1931-1956]

### What to change
Option A (minimal, in `GithubWebhook.process`):
- For `issue_comment` events, inspect payload early:
 - Skip loading for actions `edited`/`deleted`.
 - Parse the comment body for commands and only call `_load_welcome_extra_info_from_file()` if it includes `/regenerate-welcome` and/or `/reprocess` (any command that can create/update the welcome message).

Option B (cleaner separation):
- Remove unconditional load for `issue_comment` in `process()`.
- Call `_load_welcome_extra_info_from_file()` inside `PullRequestHandler.regenerate_welcome_message()` (and any path that creates the initial welcome message) right before `_prepare_welcome_comment()`.

Either way:
- Keep the existing pull_request-action gating (opened/reopened/ready_for_review), but ensure the file is loaded for any command path that can create a welcome message (not just regenerate).
- Add a unit test proving that non-welcome commands do not trigger file fetch (mock `repository.get_contents` and assert not called).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


13. Config size limit missing ✓ Resolved 🐞 Bug ☼ Reliability
Description
The runtime path that loads welcome-extra-info from config/.github-webhook-server.yaml does not
enforce the documented 10KB max, so an oversized value can bloat the welcome message and potentially
fail comment create/edit calls.
Code

webhook_server/libs/github_api.py[R1120-1122]

+        self.welcome_extra_info = self.config.get_value(
+            "welcome-extra-info", return_on_none="", extra_dict=repository_config
+        )
Relevance

⭐⭐ Medium

Team often adds runtime config validation (accepted #998/#1109), but no precedent for enforcing
schema maxLength at runtime.

PR-#998
PR-#1109

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code path added by this PR loads welcome-extra-info via Config.get_value() with no runtime
length validation, and later the handler injects the raw string into the welcome comment body.
Config.get_value itself performs no schema/maxLength enforcement, and schema-related validation code
appears only under tests.

webhook_server/libs/github_api.py[1118-1123]
webhook_server/libs/config.py[132-154]
webhook_server/libs/handlers/pull_request_handler.py[695-707]
webhook_server/tests/test_schema_validator.py[1-46]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`welcome-extra-info` is documented as max 10KB (schema + file loader), but the runtime config loader assigns the raw value directly. If schema validation is not applied at runtime, an oversized value can inflate the generated welcome message and cause GitHub API failures when creating/editing the comment.

### Issue Context
- `Config.get_value()` returns raw values and does not validate max length.
- `_prepare_extra_info_welcome_section` injects the value into the welcome comment body.

### Fix Focus Areas
- webhook_server/libs/github_api.py[1118-1123]
- webhook_server/libs/config.py[132-154]
- webhook_server/libs/handlers/pull_request_handler.py[695-707]

### What to change
- After loading `self.welcome_extra_info`, add a runtime guard:
 - If value is not a string: log warning and set to "".
 - If `len(value) > 10240`: log warning and either truncate to 10240 or clear to "" (choose one consistent policy; clearing is safer to avoid GitHub comment size errors).
- Apply the same guard regardless of whether the value came from repo YAML (`extra_dict`) or config.yaml.
- Add/extend unit tests to cover oversized config values if you choose to enforce/truncate.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

14. Missing log prefix ✓ Resolved 🐞 Bug ◔ Observability
Description
When welcome-extra-info exceeds the 10KB limit, _repo_data_from_config() logs a warning without
self.log_prefix, making it difficult to attribute the warning to a specific repo/webhook instance
in multi-repo deployments.
Code

webhook_server/libs/github_api.py[R1132-1134]

+            if _byte_len > 10240:
+                self.logger.warning(f"welcome-extra-info exceeds 10KB limit ({_byte_len} bytes). Ignoring.")
+                self.welcome_extra_info = ""
Relevance

⭐⭐⭐ High

Team frequently enforces/accepts adding log_prefix for diagnostic logs (accepted in PR #998;
traceability focus in PR #808).

PR-#998
PR-#808

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The oversize warning lacks self.log_prefix, while nearby warnings (including the file-size warning
for the markdown override) include the prefix, demonstrating an inconsistency that reduces
traceability.

webhook_server/libs/github_api.py[1121-1134]
webhook_server/libs/github_api.py[1183-1188]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The oversize warning for `welcome-extra-info` is emitted without `self.log_prefix`, unlike most other warnings in this module. This reduces diagnosability because the log line can’t be tied to a specific repo/webhook execution.

### Issue Context
Other warnings in `GithubWebhook` consistently include `self.log_prefix` (or a `getattr(self, "log_prefix", "")` fallback) so operators can correlate logs to the repository and event.

### Fix Focus Areas
- webhook_server/libs/github_api.py[1132-1134]

Suggested change: prepend the prefix, e.g.:
```python
prefix = getattr(self, "log_prefix", "")
self.logger.warning(f"{prefix} welcome-extra-info exceeds 10KB limit ({_byte_len} bytes). Ignoring.")
```
(or use `self.log_prefix` directly if always present here).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


15. Duplicate UTF-8 encode ✓ Resolved 🐞 Bug ➹ Performance
Description
In _repo_data_from_config(), the oversized welcome-extra-info guard encodes the same string to
UTF-8 twice (once for the > 10240 check and again to compute _byte_len). This is unnecessary
duplicate work and can be simplified by computing the byte length once.
Code

webhook_server/libs/github_api.py[R1130-1132]

+        elif len(self.welcome_extra_info.encode("utf-8")) > 10240:
+            _byte_len = len(self.welcome_extra_info.encode("utf-8"))
+            self.logger.warning(f"welcome-extra-info exceeds 10KB limit ({_byte_len} bytes). Ignoring.")
Relevance

⭐⭐⭐ High

Team previously accepted eliminating duplicate string operations for reuse in github_api.py (PR
#961).

PR-#961

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The referenced lines show encode("utf-8") being called in the elif condition and immediately
again to compute _byte_len for logging, duplicating the same work.

webhook_server/libs/github_api.py[1125-1133]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The oversized `welcome-extra-info` guard performs two `encode("utf-8")` calls on the same string back-to-back.

### Issue Context
This code runs when loading repo config and checking the 10KB limit.

### Fix Focus Areas
- webhook_server/libs/github_api.py[1130-1132]

### Suggested fix
Compute the UTF-8 byte length once (e.g., `byte_len = len(self.welcome_extra_info.encode("utf-8"))`) and reuse it for both the comparison and the log message.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread webhook_server/libs/handlers/pull_request_handler.py
Comment thread webhook_server/libs/github_api.py
Comment thread webhook_server/tests/test_github_api.py
Comment thread webhook_server/config/schema.yaml Outdated
Comment thread webhook_server/libs/github_api.py
Comment thread webhook_server/libs/github_api.py Outdated
@rnetser

rnetser commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author

@qodo-code-review[bot]

The following review comments were reviewed and a decision was made:

webhook_server/tests/test_github_api.py:2820 (qodo rule violation) — New tests lack type hints

Addressed: Fixed — added full type annotations (parameter types + return types) to all new test functions and fixtures in both test_github_api.py and test_pull_request_handler.py.

webhook_server/config/schema.yaml:254 (qodo requirement gap) — welcome-extra-info schema untested

Addressed: Fixed — added 4 schema validation tests covering: global valid string, global integer type violation, per-repo valid string, and maxLength violation (>10240 chars).

webhook_server/libs/handlers/pull_request_handler.py:695 (qodo requirement gap) — _prepare_extra_info_welcome_section header wrong

Addressed: By design per updated issue #1143 spec. The header uses ### 📌 Additional Information (h3 with pin emoji) to match the heading level of all other welcome message sections (Tips, Merge Requirements, Review Process). See Design Decisions section in issue #1143.

webhook_server/libs/github_api.py:1179 (qodo requirement gap) — File markdown is .strip()ed

Addressed: By design per updated issue #1143 spec. .strip() is standard file-reading behavior — leading/trailing whitespace in files is typically unintentional, and GitHub markdown renderer ignores it anyway. See Design Decisions section in issue #1143.

webhook_server/libs/github_api.py:1120 (qodo bug) — Config size limit missing

Addressed: Fixed — added runtime guard after config loading: validates type (must be str) and length (must be <=10240). Non-string or oversized values are logged as warnings and cleared to empty string. Added 3 unit tests.

webhook_server/libs/github_api.py:694 (qodo bug) — Eager file fetch on comments

Addressed: Fixed — removed issue_comment from the process() file loading guard. File is now loaded only for pull_request opened/reopened/ready_for_review events. For /regenerate-welcome, the file is loaded inside regenerate_welcome_message() before building the comment.

@qodo-code-review

Copy link
Copy Markdown

code-review[bot]

The following review comments were reviewed and a decision was made:

webhook_server/tests/test_github_api.py:2820 (qodo rule violation) — New tests lack type hints

Addressed: Fixed — added full type annotations (parameter types + return types) to all new test functions and fixtures in both test_github_api.py and test_pull_request_handler.py.

webhook_server/config/schema.yaml:254 (qodo requirement gap) — welcome-extra-info schema untested

Addressed: Fixed — added 4 schema validation tests covering: global valid string, global integer type violation, per-repo valid string, and maxLength violation (>10240 chars).

webhook_server/libs/handlers/pull_request_handler.py:695 (qodo requirement gap) — _prepare_extra_info_welcome_section header wrong

Addressed: By design per updated issue #1143 spec. The header uses ### 📌 Additional Information (h3 with pin emoji) to match the heading level of all other welcome message sections (Tips, Merge Requirements, Review Process). See Design Decisions section in issue #1143.

webhook_server/libs/github_api.py:1179 (qodo requirement gap) — File markdown is .strip()ed

Addressed: By design per updated issue #1143 spec. .strip() is standard file-reading behavior — leading/trailing whitespace in files is typically unintentional, and GitHub markdown renderer ignores it anyway. See Design Decisions section in issue #1143.

webhook_server/libs/github_api.py:1120 (qodo bug) — Config size limit missing

Addressed: Fixed — added runtime guard after config loading: validates type (must be str) and length (must be <=10240). Non-string or oversized values are logged as warnings and cleared to empty string. Added 3 unit tests.

webhook_server/libs/github_api.py:694 (qodo bug) — Eager file fetch on comments

Addressed: Fixed — removed issue_comment from the process() file loading guard. File is now loaded only for pull_request opened/reopened/ready_for_review events. For /regenerate-welcome, the file is loaded inside regenerate_welcome_message() before building the comment.

Thanks — this looks consistent with the intended fix set.

  • The config guard, file-loading flow, and welcome-message rendering all look addressed.
  • The new tests also cover the schema path for welcome-extra-info, so finding 2 appears covered as well.
  • I don’t see any remaining blockers from the diff you shared.

If the published summary still shows an active item, it’s likely just stale metadata and should clear once the review state refreshes.

@rnetser

rnetser commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author

@qodo-code-review

Please re-evaluate all remaining sticky findings against the current code.
For each finding, check if the referenced code has been fixed in subsequent commits.
Remove findings that are fully addressed. Keep any findings where the issue is still present in the code.

@qodo-code-review

Copy link
Copy Markdown

code-review

Please re-evaluate all remaining sticky findings against the current code.
For each finding, check if the referenced code has been fixed in subsequent commits.
Remove findings that are fully addressed. Keep any findings where the issue is still present in the code.

I re-checked the only remaining active sticky finding, finding 2 (welcome-extra-info schema untested).

Result

  • Fully addressed: the PR now adds schema coverage in webhook_server/tests/test_config_schema.py.
  • The new tests cover:
    • global welcome-extra-info
    • repository-level override
    • non-string input
    • max-length behavior

Conclusion

  • I don’t see any remaining sticky findings that still need action.
  • finding 2 can be considered resolved by the added tests.

@rnetser

rnetser commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author

@qodo-code-review

Please re-evaluate all remaining sticky findings against the current code.
For each finding, check if the referenced code has been fixed in subsequent commits.
Remove findings that are fully addressed. Keep any findings where the issue is still present in the code.

@qodo-code-review

Copy link
Copy Markdown

code-review

Please re-evaluate all remaining sticky findings against the current code.
For each finding, check if the referenced code has been fixed in subsequent commits.
Remove findings that are fully addressed. Keep any findings where the issue is still present in the code.

I re-checked the remaining sticky finding against the current code.

  • finding 2 is now addressed: webhook_server/tests/test_config_schema.py adds coverage for welcome-extra-info at both global and repo levels, including type and length validation.
  • I’ve removed it from the active set.

No active sticky findings remain.

Comment thread webhook_server/tests/test_config_schema.py Outdated
Comment thread webhook_server/libs/github_api.py
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 31ddf36

@rnetser

rnetser commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author

@qodo-code-review[bot]

The following review comments were reviewed and a decision was made:

webhook_server/tests/test_config_schema.py:1027 (qodo requirement gap) — welcome-extra-info tests accept invalid

Addressed: Fixed — replaced runtime-behavior tests with schema-definition tests. New tests verify the YAML schema correctly defines welcome-extra-info as type: string with maxLength: 10240 at both global and per-repo levels.

webhook_server/libs/github_api.py:1125 (qodo bug) — Size limit uses characters

Addressed: Fixed — changed size guard from len(str) (character count) to len(str.encode("utf-8")) (byte count) for accurate 10KB enforcement. Updated test uses 2-byte UTF-8 characters to verify byte-based measurement.

@qodo-code-review

Copy link
Copy Markdown

code-review[bot]

The following review comments were reviewed and a decision was made:

webhook_server/tests/test_config_schema.py:1027 (qodo requirement gap) — welcome-extra-info tests accept invalid

Addressed: Fixed — replaced runtime-behavior tests with schema-definition tests. New tests verify the YAML schema correctly defines welcome-extra-info as type: string with maxLength: 10240 at both global and per-repo levels.

webhook_server/libs/github_api.py:1125 (qodo bug) — Size limit uses characters

Addressed: Fixed — changed size guard from len(str) (character count) to len(str.encode("utf-8")) (byte count) for accurate 10KB enforcement. Updated test uses 2-byte UTF-8 characters to verify byte-based measurement.

Thanks — both items look addressed.

  • finding 1: the new tests now verify the schema definition instead of runtime behavior.
  • finding 6: the 10KB guard now uses UTF-8 byte length, so it matches the intended limit.

If you want, I can also take another pass over the remaining implemented/dismissed findings for consistency.

Comment thread webhook_server/libs/github_api.py Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 520526d

@rnetser

rnetser commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

@qodo-code-review[bot]

The following review comments were reviewed and a decision was made:

webhook_server/libs/github_api.py:1130 (qodo bug) — Duplicate UTF-8 encode

Addressed: Fixed — refactored to compute UTF-8 byte length once and reuse for both the comparison and the log message.

@qodo-code-review

Copy link
Copy Markdown

code-review[bot]

The following review comments were reviewed and a decision was made:

webhook_server/libs/github_api.py:1130 (qodo bug) — Duplicate UTF-8 encode

Addressed: Fixed — refactored to compute UTF-8 byte length once and reuse for both the comparison and the log message.

Thanks — that matches the change in webhook_server/libs/github_api.py.

  • finding 9 is now addressed: the UTF-8 byte length is computed once and reused.
  • There are no active findings left in this PR.

rnetser and others added 5 commits July 2, 2026 15:14
Signed-off-by: rnetser <rnetser@redhat.com>

Co-authored-by: PI (claude-opus-4-6) <noreply@pi.dev>
Signed-off-by: rnetser <rnetser@redhat.com>
Signed-off-by: rnetser <rnetser@redhat.com>

Co-authored-by: PI (claude-opus-4-6) <noreply@pi.dev>
Signed-off-by: rnetser <rnetser@redhat.com>
- Revert manual docs/ edits (regenerate with docsfy)
- Add load_welcome_extra_info_from_file() call in /reprocess path
- Rename _load_welcome_extra_info_from_file to public method
- Extract 10240 and filename string to module constants
- DRY mock_to_thread into autouse class fixture
- Use constant in log message and fix Generator type hint

Signed-off-by: rnetser <rnetser@redhat.com>
Assisted-by: Claude <noreply@anthropic.com>

Co-authored-by: PI (claude-opus-4-6) <noreply@pi.dev>
Signed-off-by: rnetser <rnetser@redhat.com>
- Add .github-webhook-server-welcome-message.md to stable interfaces
  list in AGENTS.md
- Extract _create_mock_github_webhook() and
  _create_mock_owners_file_handler() as module-level helpers in
  test_pull_request_handler.py to eliminate ~55 lines of duplication
- Add docstrings to all test methods in TestPrepareExtraInfoWelcomeSection
  and TestLoadWelcomeExtraInfoFromFile

Signed-off-by: Ruth Netser <rnetser@redhat.com>
Assisted-by: Claude

Co-authored-by: PI (claude-opus-4-6) <noreply@pi.dev>
Signed-off-by: rnetser <rnetser@redhat.com>
Move load_welcome_extra_info_from_file() inside the welcome comment
existence check to avoid unnecessary GitHub API call during /reprocess
when welcome comment already exists.

Signed-off-by: Ruth Netser <rnetser@redhat.com>
Assisted-by: Claude

Co-authored-by: PI (claude-opus-4-6) <noreply@pi.dev>
Signed-off-by: rnetser <rnetser@redhat.com>
@rnetser rnetser force-pushed the feat/issue-1143-welcome-extra-info branch from ac63194 to 5513e10 Compare July 2, 2026 12:14
@myakove-bot

Copy link
Copy Markdown
Collaborator

Clean rebase detected — no code changes compared to previous head (ac63194).
The following labels were preserved: verified, commented-myakove, commented-rnetser.

Comment thread webhook_server/tests/test_pull_request_handler.py
Comment thread webhook_server/libs/github_api.py
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 5513e10

Move welcome-extra-info definition to $defs section and use $ref at
both global and per-repo levels, matching the pattern used by
ai-features and security-checks. Single source of truth for
type/maxLength.

Signed-off-by: Ruth Netser <rnetser@redhat.com>
Assisted-by: Claude
Signed-off-by: rnetser <rnetser@redhat.com>
Comment thread webhook_server/config/schema.yaml
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 2c99e78

@rnetser

rnetser commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator Author

/verified

Remove 'reopened' from the action list that triggers
load_welcome_extra_info_from_file() in process(). The reopened
handler does not build a welcome comment, so the file fetch was
an unnecessary API call.

Signed-off-by: Ruth Netser <rnetser@redhat.com>
Assisted-by: Claude
Signed-off-by: rnetser <rnetser@redhat.com>
Comment on lines +696 to +701
if self.github_event == "pull_request" and self.hook_data.get("action") in (
"opened",
"ready_for_review",
):
await self.load_welcome_extra_info_from_file()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Duplicate file fetch 🐞 Bug ➹ Performance

GithubWebhook.process() preloads .github-webhook-server-welcome-message.md on pull_request
opened/ready_for_review, but PullRequestHandler.process_new_or_reprocess_pull_request() loads the
same file again when creating the welcome comment. This causes an avoidable second get_contents()
call on the common path where a welcome comment is created (including draft→ready_for_review).
Agent Prompt
### Issue description
The welcome-extra-info markdown file is fetched twice during welcome-comment creation for `pull_request` `opened` / `ready_for_review` events: once in `GithubWebhook.process()` and again in `PullRequestHandler.process_new_or_reprocess_pull_request()`.

### Issue Context
The handler already loads the file only when it is about to build a welcome message (and only when a welcome comment doesn’t exist). The earlier preload in `GithubWebhook.process()` is therefore redundant for these paths and costs an extra GitHub API call.

### Fix Focus Areas
- webhook_server/libs/github_api.py[696-701]
- webhook_server/libs/handlers/pull_request_handler.py[1975-1980]

### Implementation guidance
Choose one of:
1) Remove the preload from `GithubWebhook.process()` and rely on the handler(s) to call `load_welcome_extra_info_from_file()` only when building/regenerating the welcome message.
2) Add a small memoization flag on `GithubWebhook` (e.g., `welcome_extra_info_file_checked: bool`) so subsequent calls within the same webhook instance become no-ops.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 634d8f8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: support custom additional information in PR welcome message

3 participants